Skip to content

test: stop fixtures leaking config overrides and stop retrying a failed Prefect setup - #10329

Merged
fatih-acar merged 2 commits into
stablefrom
fac/fix-test-cache-bus-override-leak
Aug 19, 2026
Merged

test: stop fixtures leaking config overrides and stop retrying a failed Prefect setup#10329
fatih-acar merged 2 commits into
stablefrom
fac/fix-test-cache-bus-override-leak

Conversation

@fatih-acar

@fatih-acar fatih-acar commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Why

Two independent faults in backend-tests-integration, both found chasing the CI failure on
run 32238651760
(1 failed, 445 passed, 46 errors). Neither is caused by the PR that run belonged to.

1. A leaked cache override — the 1 failed

FAILED backend/tests/integration/message_bus/operations/request/test_proposed_change.py::TestProposedChange::test_run_generators_validate_requested_jobs
  - infrahub.exceptions.ResourceNotFoundError: Diff summary for pipeline <uuid> was not found in the cache

It is not flaky. It fails exactly when pytest-xdist happens to schedule
backend/tests/integration/proposed_change/test_artifact_regen_e2e.py onto the same worker,
earlier in the session:

run test_artifact_regen_e2e.py test_proposed_change.py result
32238651760 gw3 @58% gw3 @92% FAILED
32153198842 gw3 gw3 FAILED
32251034448 gw3 gw1 passed

memory_cache sets config.OVERRIDE.cache to a MemoryCache and never puts it back. The
dependency_provider.scope(build_cache, ...) unwinds on teardown, but build_cache() consults
config.OVERRIDE.cache first, so the override outlives the class that installed it and every
later get_cache() in that worker returns the dead MemoryCache.

test_run_generators_validate_requested_jobs writes the diff summary through a cache it builds
from config.SETTINGS.cache.driver (Redis) and run_generators reads it back via get_cache().
Once the override leaks, the write goes to Redis and the read goes to the leftover MemoryCache.

bus_simulator, immediately above it, leaks config.OVERRIDE.message_bus the same way. Nothing
fails from it today — a stale BusSimulator silently swallows messages rather than raising —
which is why it is worth closing now. Every other override site in the suite
(component/api/conftest.py, component/telemetry/test_tasks.py, component/git/test_sync_repository.py,
component/api/test_50_internals.py) already saves and restores; these two fixtures were the outliers.

2. A retried Prefect setup — the 46 errors

On gw0 the worker's Prefect testcontainer answered wait_for_prefect and then stopped responding
(httpx.ReadTimeout, dead events websocket) for the rest of the session. The container fault is
infrastructure. The 46 errors are ours: setup_task_manager_once recorded only success, so every
later test class retried the dead server — and since the setup does not fail fast against an
unreachable server, each retry cost the full 300s pytest timeout.

One broken container became 46 Failed: Timeout >300.0s errors across five test files, pushed the
session into its 1800s limit, and buried the original httpx.ReadTimeout under 45 identical copies.

What changed

Three commits, test-only. No production code, so no changelog fragment.

  • memory_cache / bus_simulator — save the previous config.OVERRIDE value and restore it in
    a finally, matching the neighbouring workflow_local fixture.
  • setup_task_manager_once — remember the failure alongside the success and re-raise it,
    chained, on every later call. The worker still fails, but once, in seconds, with the cause
    attached to the first error rather than the forty-sixth. except BaseException is deliberate:
    the pytest timeout raises Failed, which does not derive from Exception.
  • backend/tests/unit/helpers/test_task_manager.py — guards the once-on-success,
    no-retry-on-failure and BaseException cases. The once-per-process state moved onto a
    TaskManagerSetup object taking the setup callable as a constructor argument, so these drive it
    with recording and failing doubles rather than patching the module.

Verification

The cache leak, reproduced by running the two files in one process in the poisoning order:

INFRAHUB_DB_TYPE=neo4j uv run pytest \
  backend/tests/integration/proposed_change/test_artifact_regen_e2e.py \
  backend/tests/integration/message_bus/operations/request/test_proposed_change.py::TestProposedChange::test_run_generators_validate_requested_jobs \
  --neo4j
result
before 1 failed, 6 passedResourceNotFoundError: Diff summary for pipeline ... was not found in the cache
after 7 passed

The retry guard: the three new unit tests pass, and dropping the short-circuit from
setup_task_manager_once fails two of them.

🤖 Generated with Claude Code

@github-actions github-actions Bot added the group/backend Issue related to the backend (API Server, Git Agent) label Aug 19, 2026

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 1 file

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Shadow auto-approve: would auto-approve. Restore config overrides in test fixtures to prevent state leakage; test-only fix with no production impact.

Re-trigger cubic

@codspeed-hq

codspeed-hq Bot commented Aug 19, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 13 untouched benchmarks


Comparing fac/fix-test-cache-bus-override-leak (d28c010) with stable (c6f1fa1)

Open in CodSpeed

@fatih-acar
fatih-acar force-pushed the fac/fix-test-cache-bus-override-leak branch from 1351f0c to a0587bf Compare August 19, 2026 14:30
@fatih-acar fatih-acar changed the title test: stop the cache and message-bus fixtures from leaking their config overrides test: stop fixtures leaking config overrides and stop retrying a failed Prefect setup Aug 19, 2026
@fatih-acar
fatih-acar marked this pull request as ready for review August 19, 2026 15:12
@fatih-acar
fatih-acar requested a review from a team as a code owner August 19, 2026 15:12
Infrahub and others added 2 commits August 19, 2026 21:24
…ig overrides

`memory_cache` and `bus_simulator` each set a `config.OVERRIDE` field and never
put it back. The `dependency_provider.scope(...)` unwinds on teardown, but
`build_cache()` and `build_message_bus()` consult `config.OVERRIDE` *first*, so
the override outlives the class that installed it and every later resolution in
that xdist worker gets the previous class's throwaway adapter.

For the cache that surfaces as

    ResourceNotFoundError: Diff summary for pipeline <uuid> was not found in the cache

in `TestProposedChange::test_run_generators_validate_requested_jobs`. The test
writes the diff summary through a cache built from `config.SETTINGS.cache.driver`
(Redis) and `run_generators` reads it back via `get_cache()`. Once the override
leaks, the write goes to Redis and the read goes to the leftover MemoryCache.

It reads as flaky but it is scheduling: it fails exactly when xdist puts
`test_artifact_regen_e2e.py`, which uses `memory_cache`, on the same worker
earlier in the session. Runs 32238651760 and 32153198842 had both files on gw3
and failed; run 32251034448 had them on gw3 and gw1 and passed.

For the message bus nothing fails today — a stale BusSimulator swallows messages
instead of raising — so it is fixed here before it costs a debugging session.

Save and restore in a `finally`, matching the neighbouring `workflow_local`
fixture and every other override site in the suite.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…class

`setup_task_manager_once` recorded only success, so a Prefect test server that
came up and then stopped responding was retried by every later test class in that
xdist worker.

The retry is not cheap. The setup does not fail fast against an unreachable
server — it blocks on the API until the pytest timeout fires — so each retry cost
the full 300s. In run 32238651760 that turned one broken worker into 46
`Failed: Timeout >300.0s` errors across five test files and pushed the session
into its 1800s limit, with the original httpx.ReadTimeout buried under 45
identical copies.

Remember the failure alongside the success and re-raise it, chained, on every
later call. The worker still fails, but once, in seconds, with the cause attached
to the first error rather than the forty-sixth.

`except BaseException` is deliberate: the pytest timeout raises `Failed`, which
does not derive from `Exception`, and that is exactly the failure worth
remembering.

The once-per-process state moves onto a `TaskManagerSetup` object that takes the
setup callable as a constructor argument, so the tests drive it with recording and
failing doubles instead of patching the module — the adapter pattern the testing
guidelines ask for. `setup_task_manager_once()` keeps its signature and callers.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@fatih-acar
fatih-acar force-pushed the fac/fix-test-cache-bus-override-leak branch from a0587bf to d28c010 Compare August 19, 2026 21:31
@fatih-acar
fatih-acar requested review from a team as code owners August 19, 2026 21:31
@fatih-acar
fatih-acar changed the base branch from release-1.11 to stable August 19, 2026 21:32
@fatih-acar
fatih-acar marked this pull request as draft August 19, 2026 21:32
@fatih-acar
fatih-acar marked this pull request as ready for review August 19, 2026 22:17
@fatih-acar
fatih-acar merged commit 9ce0686 into stable Aug 19, 2026
53 of 54 checks passed
@fatih-acar
fatih-acar deleted the fac/fix-test-cache-bus-override-leak branch August 19, 2026 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

group/backend Issue related to the backend (API Server, Git Agent)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants